I have a fear of sprintf, strcpy and strcat. Perhaps we should ban them. All the current uses within the code seem harmless though - but they can go so badly wrong.

Size is a small number. Does it need to eliminate warnings?

Things like strlen, or subtraction of pointers, return a size_t, which is then passed to allocate_stack(). So I assumed that it could indeed be passed a size_t input, and that the various operations below would be able to handle it. But if it's a stack allocation, I guess that's not correct anyway, and the caller does need to sure it's only passing in "small" values.

I'll change it back, and update the callers not to cast away any large values.

@zherczeg What's the maximum amount you can pass to allocate_stack? There's one caller which passes in the callout_arg_size, which is 2 * sizeof(void*) * top_bracket. Because you can have 65536 capture groups, that's an entire 1MiB of data plus some extra few bytes.

Is the stack able to support enough for that?

It looks like I mixed this function up with sljit_alloc_memory. That is designed for small allocations.

This function should not be limited, although with very large numbers we could do a carry flag check.

I think this comment I have just added is correct.

I was worried about whether there's any case where a pattern can by 3GiB big (ie, bigger than an S32 can hold, but smaller than the U32 maximum for a pattern with link-size=4).

I can see there's some nice code elsewhere:

#define MAX_PATTERN_SIZE (1 << 30) /* Keep it positive */

("Keep it positive" clearly means, "don't use 1<<32" as the max size, but make sure it's comfortably smaller than that.)

This apparently ensures that you can't get in this situation, where you have a pattern whose compiled size is too large to fit in an int.

The store_offset arg is placed in the status->store_offsets array, which is already an int in the current code. So I think it's safe to make the argument an int (and hence avoid truncation when assigning to status->store_offsets).

That just moves the problem elsewhere though: now we need to worry about truncating to a 32-bit int in all the code which calls delayed_mem_copy_move.

@zherczeg I have added three casts here. I cannot verify whether these are OK. They are all truncation of an sljit_sw to an int.

It appears that all of the data placed inside kept_shared_srcw is within the range of an int; but private_srcw and shared_srcw are harder to check.

I'm hoping you "just know" already that these assignments are safe.

sljit_s32 could be better than int. The casts are ok, this should copy between (machine) stack and (backtracking) stack :)

Thank you very much for helping me out!

I have changed some uses of int to sljit_s32.

I think the changes to pcre2_jit_compile in this PR are now simply formatting - all the variables have the same type as before (since int and sljit_s32 are actually the same), and we've just added explicit casts for things that were previously implicit casts. So the runtime code should be identical.

I have changed some uses of int to sljit_s32.

historically (in the long obsolete PCRE1 codebase) all the interfaces used int and some might had not been updated since, so is usually safe to make this "conversion", but should also probably look for variables that were really meant to become PCRE2_SIZE, or in the case of JIT one of sljit_sw/sljit_uw.

Yes Carlo, I did look for cases where it should be PCRE2_SIZE or sljit_sw. That's what the discussion was about. I only changed these specific values to sljit_s32, which Zoltan commented on.

PCRE1 used int because, being started in 1997, it was written to the C90 standard, in which int32_t etc did not exist. (Just for the record.)